Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

consolidate exif parsing libraries - rework of timestamps #829

Merged
merged 11 commits into from
Mar 2, 2024

Conversation

grasdk
Copy link
Contributor

@grasdk grasdk commented Feb 11, 2024

Metadata Parsing

  • metadata.CreatedDate now supported for png.
  • Switched to exifr to parse timestamps and timezone offset
  • Switched to exifr for camerainfo
  • Removed ExifParser library dependency
  • Always using image-size to get image-dimensions, but has fallback to ifd0 and exif dimensions in case this fails.
    -- Rather than getting rid of image-size, it appears to be a good idea to use an independent source for image dimensions and only use tags as fallback (judging from a thread on exiftool's forums most software do not tag the dimensions of an image correctly).

I believe this comes pretty close to resolving #277

Timestamp handling
metadata.CreatedDate is now milliseconds based on UTC, where enough information was available. The value is read from the most accurate tag that is available. Preceedence is

  1. DateTimeOriginal (camera shutter close time)
  2. CreateDate (camera file creation time (typically same time as camera shutter close time)
  3. ModifyDate (as the name indicates a date of modification which is why it has lower preceedence)
  4. FileDate (filesystem date - is often updated by just copying a file or cloning from git, so worst timestamp of them all)

Offsets are pulled directly from the metadata if availalbe. Same preceedence as above. If no offset is available, but GPS is, the offset is calculated from GPS timestamp (which is always UTC by spec). If no offset is available by metadata or calculation, offset is not defined.

  • updated all test cases to reflect these timestamp changes
  • Added several test-cases for timestamps.

Examples
These changes mean that a picture taken in London and a picture taken in Sydney at the exact same millisecond will have the same CreatedDate value.
The offset (e.g. +00:00 or +01:00 for London and +10:00 or +11:00 for Sydney) has been added to the photometadata, where available. This can be used to give the user better information in the UI.

* exifr is used for most tags now

* New timestamp handling, removed exif-parser, date supported for png

* Removed offset from testhelper. It's optional

* explanations

* Feature/timestamps (#3)

* preparing for further timestamp test

* Added more test and fixed offset calculation bug

* Revered old dimension test, added new timestamp tests, some bug fixes

* Renamed png-test because faces overrule keywords
@grasdk
Copy link
Contributor Author

grasdk commented Feb 11, 2024

I realize that the timestamp handling change may impact users of pigallery who live far away from UTC. Without this change, dates are parsed as local time on the server that runs pigallery, that is how Javascript Date works. To amend it, I have made the offset time available in the metadata, but it has not yet been used. Until the time that the UI can utilize this offset, this change will cause a daytime photo taken in e.g. Sydney to show up with a night time (or evening or morning) timestamp in Europe and vice versa. However for the overall correctness, I believe that this approach is correct.

@grasdk grasdk marked this pull request as ready for review February 11, 2024 15:16
@grasdk grasdk marked this pull request as draft February 11, 2024 16:00
@grasdk grasdk marked this pull request as ready for review February 11, 2024 21:54
grasdk and others added 2 commits February 13, 2024 23:00
Added recognition of the offset value in the UI. It will be displayed if available.

Caveat: Search will not take offset into account. A new year's picture taken in Sydney the 1st of January 2019 00:15:00 GMT+11, is technically taken in 31st of December 2018 in UTC. Therefore this picture won't show of in seaches where the after: parameter is set to 1st of january 2019.

This is both correct and wrong at the same time.

UTC-wise it is correct, local time it is not correct. I guess most people would find local time most untuitive, so there is room for improvement of the search. :)
@grasdk
Copy link
Contributor Author

grasdk commented Feb 16, 2024

I realize that the timestamp handling change may impact users of pigallery who live far away from UTC. Without this change, dates are parsed as local time on the server that runs pigallery, that is how Javascript Date works. To amend it, I have made the offset time available in the metadata, but it has not yet been used. Until the time that the UI can utilize this offset, this change will cause a daytime photo taken in e.g. Sydney to show up with a night time (or evening or morning) timestamp in Europe and vice versa. However for the overall correctness, I believe that this approach is correct.

Most of these changes have now been mitigated with updates to the UI. Offset will be displayed and taken into account when available.

One caveat: Search using before and after date will occasionally miss a picture, because search is done by UTC time. So a new-year photo from Australia early the 1st of January, will technically still be 31st of December UTC, so it won't show up when using after: and first of January.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@grasdk
Copy link
Contributor Author

grasdk commented Feb 16, 2024

By the way. The photos added for demo and test are from https://unsplash.com/ and free to use for any purpose except reselling or competing with unsplash.

@grasdk
Copy link
Contributor Author

grasdk commented Feb 16, 2024

Hey @bpatrik

When you get to review this, which is a combined metadataloader refactor and use of timestamps with offsets. You may want to take a look at this small additional change, which I have ready for merge aswell, if you like it:

https://github.com/grasdk/pigallery2/compare/master...grasdk:pigallery2:feature/location_info?diff=split&w=

In the lightbox and infopanel for photos, it will show all of the location metadata: city, state and country separated by comma. Any empty value will be left out of course.

My reasoning for suggesting this change is programs like GeoSetter and GetTagNinja fill out these values, so you can display "Los Angeles, California, USA" or "Birmingham, England, Great Britain" for example. Many countries don't use the "State" attribute. There it will just be like "Athens, Greece" for example.

I can make it a part of this pull request or make another pull request if you think it's a good idea.

No rush ;)

Kind regards.

@bpatrik
Copy link
Owner

bpatrik commented Feb 20, 2024

I realize that the timestamp handling change may impact users of pigallery who live far away from UTC. Without this change, dates are parsed as local time on the server that runs pigallery, that is how Javascript Date works. To amend it, I have made the offset time available in the metadata, but it has not yet been used. Until the time that the UI can utilize this offset, this change will cause a daytime photo taken in e.g. Sydney to show up with a night time (or evening or morning) timestamp in Europe and vice versa. However for the overall correctness, I believe that this approach is correct.

Most of these changes have now been mitigated with updates to the UI. Offset will be displayed and taken into account when available.

One caveat: Search using before and after date will occasionally miss a picture, because search is done by UTC time. So a new-year photo from Australia early the 1st of January, will technically still be 31st of December UTC, so it won't show up when using after: and first of January.

Just to double check. After these change, this means that If I take a photo at noon (12:00) in London and someone else takes a photo 5 hours later at noon (12:00) in New York. Both of them would be shown in the app as they would have been created at the same time at noon. Even if I watch the photos in Sydney time.

That would be the expected behavior as when you look back to your holidays photos, you do not expect to see a lunch photo taken at 2am.

One future feature I can imagen is to add a switch to the lightbox menu that switches between local time and UTC (or however we would call it)

Copy link
Owner

@bpatrik bpatrik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its fine if the search update is not part of this PR.

But can you at least add a \TODO with some explanation to the right places in the file.
One sample place:

`media.metadata.creationDate ${relation} :from${queryId}`,

src/common/entities/PhotoDTO.ts Show resolved Hide resolved
src/frontend/app/ui/gallery/navigator/sorting.service.ts Outdated Show resolved Hide resolved
@bpatrik
Copy link
Owner

bpatrik commented Feb 20, 2024

Also thank you very much. This is huge contribution to the app!

@grasdk
Copy link
Contributor Author

grasdk commented Feb 21, 2024

I realize that the timestamp handling change may impact users of pigallery who live far away from UTC. Without this change, dates are parsed as local time on the server that runs pigallery, that is how Javascript Date works. To amend it, I have made the offset time available in the metadata, but it has not yet been used. Until the time that the UI can utilize this offset, this change will cause a daytime photo taken in e.g. Sydney to show up with a night time (or evening or morning) timestamp in Europe and vice versa. However for the overall correctness, I believe that this approach is correct.

Most of these changes have now been mitigated with updates to the UI. Offset will be displayed and taken into account when available.
One caveat: Search using before and after date will occasionally miss a picture, because search is done by UTC time. So a new-year photo from Australia early the 1st of January, will technically still be 31st of December UTC, so it won't show up when using after: and first of January.

Just to double check. After these change, this means that If I take a photo at noon (12:00) in London and someone else takes a photo 5 hours later at noon (12:00) in New York. Both of them would be shown in the app as they would have been created at the same time at noon. Even if I watch the photos in Sydney time.

That would be the expected behavior as when you look back to your holidays photos, you do not expect to see a lunch photo taken at 2am.

One future feature I can imagen is to add a switch to the lightbox menu that switches between local time and UTC (or however we would call it)

Yes. Lightbox etc. where time is displayed for human viewing, will show 12:00Z in London and 12:00-05:00 for New York (the default for Datepipe formatting is that UTC+00:00 is displayed at Z). This is regardless of where Pigallery is running.

Only when absolutely no information about offset is available, the local time of the pigallery2 server will be used. So if your pigallery is running in timezone +1 and you show a picture from New York without offset. It would probably show the time wrong by 6 hours (the difference between the -5 and +1 offset). That is how it works now without this pull request, if my experiments are correct. Most modern photos won't have this problem, but incomplete metadata from old cameras, scanned photos may not have saved any timezone info. But I would suggest to the owner of such photos to fix this using exiftool or one of the other tools available for this :)

Sure. A feature "show timestamps as UTC" could be made, that would replace all offsets with the fixed offset value +00:00. That's the beautiful thing about how you kept the MS format for storing timestamps. It's global, so applying offset is just a matter of how to show the timestamp in the places you want to show it.

* effective storage of offset

* added comments to searchmanager.ts fixed linting error in utils
@grasdk
Copy link
Contributor Author

grasdk commented Feb 21, 2024

I think its fine if the search update is not part of this PR.

But can you at least add a \TODO with some explanation to the right places in the file. One sample place:

`media.metadata.creationDate ${relation} :from${queryId}`,

Added a big TODO explanation here, and a bunch of smaller ones.
https://github.com/bpatrik/pigallery2/pull/829/files#diff-f85ded2f38ddf78f5cb870136c6928fe28a9532edf5ceaaee365c8120ad8ec29R566-R571

@grasdk grasdk marked this pull request as draft February 21, 2024 22:57
@grasdk grasdk requested a review from bpatrik February 21, 2024 22:57
@grasdk grasdk marked this pull request as ready for review February 26, 2024 06:00
@bpatrik
Copy link
Owner

bpatrik commented Mar 2, 2024

Great! Thank you very much for your contribution! I think I can go ahead to merge it.

@bpatrik bpatrik merged commit 9a90a15 into bpatrik:master Mar 2, 2024
5 of 8 checks passed
@bpatrik bpatrik added this to the Next (probably v2.5) milestone Mar 2, 2024
@bpatrik
Copy link
Owner

bpatrik commented Mar 2, 2024

ahh this PR ended up breaking tests :( https://github.com/bpatrik/pigallery2/actions/runs/8125512654/job/22208202972

for search date pattern tests

@grasdk
Copy link
Contributor Author

grasdk commented Mar 3, 2024

Ok. I will take a look asap. I will prioritize this as much as possible.

kagahd added a commit to kagahd/pigallery2 that referenced this pull request Mar 4, 2024
@grasdk
Copy link
Contributor Author

grasdk commented Mar 5, 2024

Hi @bpatrik
I finally figured out what the problem is. It's leap year and we're past the 29th of february. The tests that fail compare day of year this year with day of year other years. Today (march 5th) is 65th day of 2024, but only 64th day of year 2023. Hence we have some test data that isn't properly picked up :)

It also explains why I didn't get the error in the pull request build, because it was before the 29th of february.

I'm thinking that replacing the format string "%j" (dayofyear) with "%m%d" (month and date) will help. I will try this out and report back (hopefully with a pull request that has a fix)

@bpatrik
Copy link
Owner

bpatrik commented Mar 6, 2024

I'm sorry, but apparently there were not enough tests on metadataloader and the timestamps for some photos are borken now in the demo folder. These photos have a proper date:

kép
From: https://pigallery2.onrender.com/gallery/%2F

@bpatrik
Copy link
Owner

bpatrik commented Mar 6, 2024

Here is a quick var console.log dump. I will let this with you as I suspect the issue is with the new exifr part:

Input file: wild-1.jpg from the demo folder.

stat.mtime.getTime: 1694635419042
iptcData.date_time.getTime: 1435917883000
exif.exif: {                                                   
  ExifVersion: Uint8Array(4) [ 48, 50, 51, 49 ],    
  CreateDate: '2015:07:03 10:04:43',                
  OffsetTime: '+02:00',                             
  SubSecTimeDigitized: '50',                        
  ColorSpace: 1,                                    
  ImageUniqueID: '5256C01EA869151BFA5D760D7D06D6B1',
  SerialNumber: '123063022888',                     
  LensInfo: [ 15, 85, NaN, NaN ],                   
  LensModel: 'EF-S15-85mm f/3.5-5.6 IS USM',        
  LensSerialNumber: '0000129324'                    
}       
output:                                            
{                                                                                
  size: { width: 1224, height: 816 },                                            
  creationDate: 0,                                                               
  fileSize: 632861,                                                              
  positionData: { country: 'United States', state: 'Arizona', city: 'Williams' },
  keywords: [ 'Bearizona', 'USA', 'USA Road trip' ],                             
  cameraData: {                                                                  
    make: 'Canon',                                                               
    model: 'Canon EOS 600D',                                                     
    lens: 'EF-S15-85mm f/3.5-5.6 IS USM'                                         
  },
  creationDateOffset: '+02:00',
  faces: [
    { name: 'Balu the bear', box: [Object] },
    { name: 'Hugin the raven', box: [Object] },
    { name: 'Akela the wolf', box: [Object] }
  ]
}

The 'solution' should be '2015:07:03 10:04:43.

@bpatrik
Copy link
Owner

bpatrik commented Mar 6, 2024

Maybe this should be exif.exif.CreateDate?

metadata.creationDate = timestampToMS(exif.exif.DateTimeOriginal, alt_offset);

pls doublecheck.

kagahd added a commit to kagahd/pigallery2 that referenced this pull request Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants